chore: Move to base's next #859
Conversation
igamigo
added
no changelog
igamigo
force-pushed
the
igamigo-move-to-next
branch
from
e0b8f14 to
ca5a099
Compare
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comments inline - but these are more for future discussions/considerations.
| /// Account state headers. | ||
| #[derive(Clone, Debug)] | ||
| pub struct StateHeaders { | ||
| // TODO: should this be renamed? or storage_slots moved to AccountProof |
There was a problem hiding this comment.
Yeah, naming here could be improved. Basically, we have:
AccountWitnesswhich proves that an account with a given commitment is in the chain.StateHeaderswhich contains some information about the account, but not all info.AccountProofwhich containAccountWitnessand optionalStateHeaders.
I think I'm OK with AccountWitness but the naming of StateHeaders and AccountProof is confusing.
Maybe StateHeaders should be something like PartialAccount as it contains partial information about the account.
Not sure what would be a good name for AccountProof though.
There was a problem hiding this comment.
Created an issue to track this (#862), though I think I might be able to refactor some of this from miden-base (since it's related to 0xMiden/miden-base#1229)
| let account_leaf = account_witness.leaf(); | ||
| let account_leaf_hash = account_leaf.hash(); | ||
|
|
||
| // Extend the advice inputs with Merkle store data | ||
| tx_args.extend_merkle_store( | ||
| merkle_path.inner_nodes(account_id.prefix().as_u64(), account_header.commitment())?, | ||
| account_witness | ||
| .path() | ||
| .inner_nodes(account_id.prefix().as_u64(), account_leaf_hash)?, | ||
| ); | ||
| // populate advice map with the account's leaf | ||
| tx_args.extend_advice_map([(account_leaf_hash, account_leaf.to_elements())]); |
There was a problem hiding this comment.
Not for this PR, but I wish this was somehow abstracted away - e.g., AccountWitness "knew" how to add itself to tx args. But that's probably a bigger change throughout the codebase.
There was a problem hiding this comment.
Seems also a bit related to 0xMiden/miden-base#1229 (comment)
3 participants
Refactors FPI-related changes after changes in witness data on base and node.